Repository navigation
Conversation
The `TransformSession` constructors took a `&Settings` and passed
`Settings::serialize_schema()` as the session options. That is the
settings schema (groups, titles, types, defaults), not the values set on
the instance, and the core expects a JSON object of setting keys and
values, which is what Python and C++ send. The core ignores the schema,
so per-session overrides such as `files.container.excludedTransforms`
had no effect from Rust.
`new`, `new_with_mode`, `from_view`, `from_view_with_mode` and
`from_context` now take `options: Option<O>` with `O: IntoJson`, the same
as `load_with_options`, and send `{}` for `None`. Like
`load_with_options`, they return `None` if the options can't be
converted to JSON.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The five Rust
TransformSessionconstructors (new,new_with_mode,from_view,from_view_with_mode,from_context) take a&Settingsand sendsettings.serialize_schema()as the sessionoptions. That string is the settings schema (groups, titles, types, defaults), not the values set on the instance. The core expects a JSON object of setting keys and values, which is what Python (json.dumps(options)) and the C++ default ("{}") send. The core ignores the schema, so per-session overrides such asfiles.container.excludedTransformshave no effect from Rust.On
dev, with a ZIP of two files and an override instance that excludes theZiptransform:The override should give
transform="" children=0.Change
new,new_with_mode,from_view,from_view_with_modeandfrom_contextnow takeoptions: Option<O>withO: IntoJson, likeload_with_options.Nonesends{}. They returnOption<Ref<TransformSession>>:Noneif the options can't be converted to JSON or the core returns no session, asload_with_optionsdoes. The doc ofnewdescribes the options format and has an example.This is a breaking change. Callers pass
None::<&str>orSome(json)instead of&settingsand unwrap the result. The only caller in this repository,test_context_transform, is updated. I chose it over keeping&Settingsbecause it matchesload_with_optionsand the Python and C++ APIs. Keeping&Settingswould need a wrapper forBNSerializeSettings(SettingsAutoScope), and per-session options would then have to live in a namedSettingsinstance that is shared process-wide and carries values between sessions.Testing
New
rust/tests/transform_session.rsbuilds a two-file ZIP at test time, so there is no binary fixture.test_session_optionschecks that withNone,new_with_modeandfrom_view_with_modeextract the ZIP (root transformZip, 2 children), and that with{"files.container.excludedTransforms": ["Zip"]}they don't (no transform, 0 children). It passes on this branch.The tests are in their own file because
rust/tests/transform.rsregistersMyDetectionTransform, which accepts any input of 4 bytes or more and would be applied to these sessions. Ondev, running all ofcargo test --test transformtogether already overflows the stack intest_context_transformfor that reason; each test passes when run alone. This PR doesn't change that.from_contextsends the options too, but a session created from an existing context used that context's settings in my tests, so it isn't covered.cargo fmt --checkandcargo clippy -p binaryninja --all-features --lib --test transform --test transform_sessionare clean.